@findepi looks like one of the CI jobs timed out. would it be possible to restart just this one job?

@nastra GitHub Actions recently introduced "Re-run failed jobs". The feature might be not yet stable though. By the way, we will request to push an empty commit for triggering CI if necessary.

@ebyhr I don't have permissions unfortunately to re-run the failed job myself, that's why I closeed & reopened the PR, which triggers a full CI run again (same as pushing)

Warning:  Invalid bytecodeVersion for org.glassfish.jersey.core:jersey-common:jar:2.30:compile : META-INF/versions/11/org/glassfish/jersey/internal/jsr166/SubmissionPublisher$1.class: expected 55, but was 56

This library seems to be compiled with Java 12, but Trino works with Java 11

@findinpath I can see the same warning in other CI runs on other branches. Here's an example from master from https://github.com/trinodb/trino/runs/5763019856?check_suite_focus=true.

Nessie itself is also compiled with Java 11 btw

The tests should be normal unit tests, not ITs. You can probably use Testcontainers for Nessie.

It would be nice to see some tests which involve dealing with running SQL statements over the query runner to actually test how the connector works with the new catalog type.

Ideal would be (at a later point) to have additionally also a product tests environment (similar to singlenode-spark-iceberg
See for reference:

testing/bin/ptl env describe  --environment EnvSinglenodeSparkIceberg  --config config-default

Thank you @nastra for your contribution. I am looking forward to help out in this PR to add support for Nessie in Trino Iceberg connector.

@findinpath thanks for the review. I updated the code based on your review and pushed a new commit. Will look into doing some additional testing via SQL statements now.

@nastra you can use the Glue related tests from the connector as a rough template on what kinds of tests you need to add.

Could you please add documentation on how to setup Nessie catalog in the Iceberg connector

You can use https://github.com/trinodb/trino/pull/11772/files#diff-e1aabf1cfd8bd8aa7d1b75e70089b57413b2e620a5eebeb36cc76fd3f2ac60db as a template for getting started.

The REST catalog is fine and dandy, but I would prefer nessie. Tabular.io's iceberg-rest isn't production.

Hey @Cerebus. Currently, the primary focus is on the JDBC catalog, but let's see if @findepi has any initial comments.

Hi @bitsondatadev , can you help us getting this PR reviewed approved, It's great that the Iceberg REST catalog got merged, as Nessie is an open source project with support in many query engines we would like to add support for Nessie into Trino as well.

PR is rebased and ready for the review. Thanks @nastra

@findinpath can you give this another look?

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@electrum, @findinpath, @ebyhr, @findepi:
Hi, Can we please have another look at this and help in merging this? We are getting quite a few requests for the Trino Iceberg connector supporting the Nessie catalog these days.

@electrum, @findinpath, @ebyhr, @findepi, @ajantha-bhat:
any news?

Hi Trino community, It's been more than a year since this PR has been opened. All of the review comments have been addressed and it has been rebased many times. Can we get some feedback from the core team as to why this PR is not approved?

The Nessie catalog is a core Iceberg catalog and is implementing the Iceberg Catalog API.

@nastra Can you rebase this, then I'll merge it

PR is rebased. Thanks @nastra

@electrum anything else needed to get this across the finish line?

I think we should re-review this PR. For instance, iceberg.nessie.ref config property name should be changed to iceberg.nessie-catalog.ref for consistency with other catalogs.

93 successful, 2 skipped, 1 cancelled, and 1 failing checks

io.trino.plugin.hive.TestHiveConnectorTest.testWriterTasksCountLimitPartitionedScaleWritersEnabled Time elapsed: 0.157 s <<< FAILURE!

It seems the failure is unrelated to the new changes and also it is in the hive connector (changes are for the iceberg connector).
This test case could be flaky.

Closing and re-opening the PR to retrigger the build.

@ebyhr: Thanks for the review.

Fixed all the comments. PR is ready for review.

The failed test TestHiveStorageFormats > testSelectWithNullFormat(0: StorageFormat{name=SEQUENCEFILE, properties={}, sessionProperties={}}) [groups: storage_formats_detailed] is independent of PR changes and could be flaky.

Re-triggering the build

@ajantha-bhat Please note that you don't need to retrigger CI when the failed test is unrelated to the change. Also, we recommend pushing an empty commit instead of reopening PR when retriggering CI.

@ajantha-bhat Please note that you don't need to retrigger CI when the failed test is unrelated to the change. Also, we recommend pushing an empty commit instead of reopening PR when retriggering CI.

Thanks. I am new to Trino contributions. Good to know.

PR is ready. Thanks.